Skip to content

replacing codeflask with code-input #2139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 47 commits into from

Conversation

Bowrna
Copy link
Contributor

@Bowrna Bowrna commented Nov 5, 2024

related: #2072

This is work in progress to replace codeflask with code-input for showing syntax highlighted text/code editor inside campaign content.

@Bowrna
Copy link
Contributor Author

Bowrna commented Nov 19, 2024

I have some css related issue in this PR. Let me try debugging and closing this issue before this weekends.

@Bowrna Bowrna marked this pull request as ready for review December 27, 2024 17:09
@Bowrna
Copy link
Contributor Author

Bowrna commented Jan 2, 2025

Could this PR be reviewed?
If there is any feedback, let me know.

@knadh
Copy link
Owner

knadh commented Jan 5, 2025

Hi @Bowrna. There seems to be several issues with this.

The HTML editor is completely broken. This is what I'm getting:
image

Referring ./node_modules/* in the frontend HTML file is not correct. All modules should be bundled in the JS compilation.

@Bowrna
Copy link
Contributor Author

Bowrna commented Jan 6, 2025

@knadh Let me fix this and update here. I missed this.

this.initHTMLEditor(this.$props.value || '');
},

watch: {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the below code being used only in markdownEditor and not in HTMLeditor?

watch: {
    value(newVal) {
      if (newVal !== this.data) {
        this.data = newVal;
      }
    },
  },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me check this one @Et-008

@Bowrna Bowrna force-pushed the replace_codeflask branch from 80e6dd1 to 24d6710 Compare March 6, 2025 09:30
pbowrna and others added 5 commits March 24, 2025 23:02
Bumps [prismjs](https://github.com/PrismJS/prism) from 1.29.0 to 1.30.0.
- [Release notes](https://github.com/PrismJS/prism/releases)
- [Changelog](https://github.com/PrismJS/prism/blob/master/CHANGELOG.md)
- [Commits](PrismJS/prism@v1.29.0...v1.30.0)

---
updated-dependencies:
- dependency-name: prismjs
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@Bowrna Bowrna force-pushed the replace_codeflask branch from f2ff6c5 to 06c0c5f Compare April 16, 2025 08:57
dependabot bot and others added 13 commits April 16, 2025 14:40
Bumps [github.com/go-jose/go-jose/v3](https://github.com/go-jose/go-jose) from 3.0.3 to 3.0.4.
- [Release notes](https://github.com/go-jose/go-jose/releases)
- [Changelog](https://github.com/go-jose/go-jose/blob/main/CHANGELOG.md)
- [Commits](go-jose/go-jose@v3.0.3...v3.0.4)

---
updated-dependencies:
- dependency-name: github.com/go-jose/go-jose/v3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.33.0 to 0.36.0.
- [Commits](golang/net@v0.33.0...v0.36.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
knadh and others added 19 commits April 16, 2025 14:40
- Fix status/button state management issues when `Send at` was toggled
  under various scenarios.
- Allow paused campaigns to be edited and turned into scheduled campaigns.
- Add Cypress UI tests for unscheduling.
This patch introduces new `campaigns:get_all` and `campaigns:manage_all`
permissions which alter the behaviour of the the old `campaigns:get` and
`campaigns:manage` permissions. This is a subtle breaking behavioural change.

Old:

- `campaigns:get` -> View all campaigns irrespective of a user's list
  permissions.
- `campaigns:manage` -> Manage all campaigns irrespective of a user's list
  permissions.

New:

- `campaigns:get_all` -> View all campaigns irrespective of a user's list
  permissions.
- `campaigns:manage_all` -> Manage all campaigns irrespective of a user's list
  permissions.
- `campaigns:get` -> View only the campaigns that have at least one list to
  which which a user has get or manage access.
- `campaigns:manage` -> Manage only the campaigns that have at list one list
  to which a user has get or manage access.

In addition, this patch refactors and cleans up certain permission related
logic and functions.
- Move user models from `/models` to `internal/auth`.
- Move and refactor various permission check functions into `User.()`
- Refactor awkward `get, manage bool` function args into `Get|Manage` bitflags.
- Make the beginning of handlers consistent with uniform variable declaration
  and grouping.
- Add missing comments.
- Fix staticcheck/vet warnings and idiom issues.
- Attach all HTTP handlers to a new `Handlers{}` struct.
- Remove all `handle*` function prefixes.
- Remove awkward, repetitive `app = c.Get("app").(*App)` from all handlers
  and instead, simply access it from `h.app` from `Handlers{}`

Originally proposed in knadh#2292.
`App{}` now is used purely as a container for HTTP handlers.
This patch significantly cleans up clunky, repetitive, and pervasive
validation logic across HTTP handlers.

- Rather than dozens of handlers checking and using strconv to validate ID,
  the handlers with `:id` are now wrapped in a `hasID()` middleware that does
  the validation and sets an int `id` in the handler context that the wrapped
  handlers can now access with `getID()`.

- Handlers that handled both single + multi resource requests
  (eg: GET `/api/lists`) with single/multiple id checking conditions are all now
  split into separate handlers, eg: `getList()`, `getLists()`.
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 5.4.15 to 5.4.17.
- [Release notes](https://github.com/vitejs/vite/releases)
- [Changelog](https://github.com/vitejs/vite/blob/v5.4.17/packages/vite/CHANGELOG.md)
- [Commits](https://github.com/vitejs/vite/commits/v5.4.17/packages/vite)

---
updated-dependencies:
- dependency-name: vite
  dependency-version: 5.4.17
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…knadh#2322.

- During install, listmonk now accepts the env `LISTMONK_ADMIN_API_USER`
  and creates an API user (with username $LISTMONK_ADMIN_API_USER)
  with full superadmin permissions. This requires LISTMONK_ADMIN_USER and
  LISTMONK_ADMIN_API_PASSWORD to be set so that that there's always a superadmin
  user to avoid bad states, mainly: bot superadmin exists, but no admin user
  exists, leaving the installation perpetually open with the superadmin user
  creation UI on the first login.
  The API user's token is printed to stderr in the following format:
  `export LISTMONK_ADMIN_API_TOKEN="7I81VSd90UWhKDj5Kq9c6YopToRduyDF"`
  This can be redirected to a file with ./listmonk 2> /tmp/token or captured
  directly and then source()'d.
- Add new function `core.GetRole(id)`.
- Fix `at least one super admin` query in user deletion.
@Bowrna Bowrna force-pushed the replace_codeflask branch 2 times, most recently from 9f339f7 to aea61ce Compare April 16, 2025 09:14
@Bowrna Bowrna force-pushed the replace_codeflask branch from aea61ce to 2b9732c Compare April 16, 2025 09:21
@Bowrna
Copy link
Contributor Author

Bowrna commented Apr 16, 2025

I messed up this PR during rebase. Closing this one and raising another PR

@Bowrna Bowrna closed this Apr 16, 2025
@knadh
Copy link
Owner

knadh commented Apr 20, 2025

@Bowrna, this is now resolved in ba5e77b.

Last week, I spent some time looking at CodeInput, and not only is it tedious to integrate, but has a bunch of deal-breaker open issues. I went ahead and integrated CodeMirror (which is a much larger lib), which is battle tested and was actually a breeze to integrate.

Thanks again for the PR.

@Bowrna
Copy link
Contributor Author

Bowrna commented Apr 21, 2025

Thanks for this update @knadh
I had a tough time with this task. Could you share the deal-breaker open issues that you faced regarding CodeInput? Integration was tedious, and while I faced multiple issues, I was unsure if I could call out the issues, and I kept trying to fix them. If you could share about these deal-breaker issues, it would be helpful for me to understand the thought behind this decision, and I can learn to navigate through such scenarios.

@knadh
Copy link
Owner

knadh commented Apr 21, 2025

CodeInput is unfortunately not packaged in clean, importable, reusable way. Or packaged at all.

  • You can't yarn/npm install it and import and use it in the app directly. The only way seems to be to add build steps in the Makefile to copy the bundles from the ./node_modules directory into the frontend/dist directory. This is not nice.
  • Since it's not importable, there's no way to integrate it seamlessly. When the editor view renders, have to do DOM manipulation to dynamically inject a <script> tag pointing to the dist, wait for it to load, and then init. Or, have a dedicated page that does all of this and <iframe> it in the editor, then do hacky iframe manipulation. Also not nice.
  • Plugins seem to be outright non-installable via yarn/npm. The repo has them committed, but they don't seem to be packaged.
  • Multiple fundamental open issues including broken RTL, cursor placement, text alignment, and scroll issues.

Too many bad trade-offs. The better trade-off was to just bite the bullet and use CodeMirror, a much bigger, but cleaner, and robust library.

CodeMirror integration is seamless. Install the necessary modules, and simply import, compose, and use them.
https://github.com/knadh/listmonk/blob/feat-visual-editor/frontend/src/components/CodeEditor.vue

@Bowrna
Copy link
Contributor Author

Bowrna commented Apr 22, 2025

Thanks for sharing this insight @knadh. Yes, I couldn't install CodeInput through the package manager. Faced issues with wrong cursor alignments, styling mismatches that make HTML tags break the lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.